-
Notifications
You must be signed in to change notification settings - Fork 6
feat(auth): automatically fetch Columnar license on login #223
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Nice! Let's also please make it so that if the user runs
If all three of those things are true, dbc should download |
e424e54 to
052c212
Compare
052c212 to
f59154c
Compare
|
@ianmcook updated per your suggestion |
|
Added @lidavidm so he can see where dbc is putting the license |
Nice! I tested it successfully. |
lidavidm
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you log in and your trial is expired, you don't get any feedback:
$ dlv debug --headless ./cmd/dbc -- auth login
API server listening at: 127.0.0.1:39367
Opening https://auth.columnar.tech/activate?user_code=QVLR-GDCV in your default web browser...
Gtk-Message: 13:10:44.101: Not loading module "atk-bridge": The functionality is provided by GTK natively. Please try to not load it.
Authentication successful!
In dlv, I can see that I got a 403:
> github.com/columnar-tech/dbc/auth.FetchColumnarLicense() ./auth/credentials.go:318 (PC: 0x978c8a)
313: if err != nil {
314: return err
315: }
316: defer resp.Body.Close()
317:
=> 318: if resp.StatusCode != http.StatusOK {
319: return fmt.Errorf("failed to fetch license: %s", resp.Status)
320: }
321:
322: licenseFile, err := os.OpenFile(licensePath, os.O_CREATE|os.O_TRUNC|os.O_RDWR, 0o600)
323: if err != nil {
(dlv) p resp.StatusCode
403
But this is not reported in the UI.
e5e59dc to
c627606
Compare
|
I'm not sure if this is the same error as @lidavidm got but here's what I did:
Back in my terminal, the CLI is stuck on
I note that my trial is expired. Also I notice that the text says
but it should probably tell me that my license is expired. |
74fdb80 to
f814d75
Compare
|
@amoeba can you try again? It looks good to me (and I'll update the website to be more explicit when the trial is expired) |
Co-authored-by: Bryce Mecum <[email protected]>
|
Thanks @zeroshade. I can't reproduce anymore as I don't have an expired license but |
| var cmd tea.Cmd | ||
| switch { | ||
| case errors.Is(msg, auth.ErrTrialExpired): | ||
| cmd = tea.Println(errStyle.Render("Could not download license, trial has expired")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
set non-zero status? I'm not totally sure if we should or not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My personal opinion is that we don't need a non-zero status for the trial handling because I don't want dbc to always fail if they login but haven't started the trial yet etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
okay, after discussing with @ianmcook we decided that this will give a non-zero status, but it won't error/give a non-zero status during dbc auth login. It'll only error during install/sync
I've updated this accordingly
8ccf3c3 to
f857861
Compare
|
I ran into an issue while testing driver list support for this, This was |
|
I ran dbc auth login and it worked after I did that. I can't remember how I got the token that 401's but, looking at it decoded, the biggest difference is that,
Otherwise things looked structurally similar. |
|
sigh yea, looks like I might have to do part of what I was discussing with Ian. I was trying to reduce calls and latency but I guess there's no way around it. I'll do some stuff tomorrow to make this more robust and handle things better instead of relying on the (potentially stale) login token |
|
@amoeba Okay, I've updated the lambda func to fallback and make a call to heimdall.columnar.tech for the latest token when the token came from Auth0 and doesn't contain the appropriate permissions. This will cover the scenario you had where you had to re-login. |
|
Nice! Let me test that out in a sec and I'll report back. |
|
I wasn't able to get this to work how I expected. I deleted my account in auth0, signed up with a fresh account, logged in but did not start my trial, ran |
|
I had mitmproxy running dbc made two requests (amongst others) to install the driver. The first 401 had a JWT with |
|
Maybe something else is going on. I went back to the preview Cloud site and the UI didn't show I had a trial. I then tried to restart my trial and it kept failing after I hit Accept with a 400 "Trial already started" in the console. I'll file a bug since the registry site needs to handle that. |
When using
dbc auth loginto login to the Columnar private registry, we'll check the token and see if it contains thetrial_startclaim, downloading the license if it exists.There's some more work that needs to happen here so I'm leaving this marked as WIP, such as: